feat: rework manager api for cross-major calls - #8
Open
kp2pml30 wants to merge 1 commit into
Open
Conversation
📝 WalkthroughWalkthroughThe change adds boolean literals to the expression language and expands nested execution with delegated routing, permissions, resource budgets, recursion tracking, host RPCs, result hashing, atomic precompile writes, and related build and tooling updates. ChangesRuntime and protocol changes
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant VM
participant Host
participant NestedExecutor
VM->>Host: resolve_callcontract_executor
Host-->>VM: return routing payload
VM->>Host: run_nested with NestedRunEnvelope
Host->>NestedExecutor: forward nested execution
NestedExecutor-->>Host: return NestedRunReply
Host-->>VM: return validated nested result
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
kp2pml30
force-pushed
the
pr/v0.3/feat/rework-manager-api
branch
2 times, most recently
from
July 29, 2026 12:03
c0e3915 to
30fcc9e
Compare
* feat(sdk): call contracts of another major through the host ✨ * refactor(rt): bound recursion with a budget minted by the chain root ♻️ * refactor(exe): carry nested execution state as one explicit group ♻️ * fix(exe): fold the callee's small hash on the nested route 🐛🔒️ * feat(exe): refuse a crossing call while custom runners are loaded ✨ * fix(exe): hand a major this line cannot serve to the manager 🐛 * feat(host): write caller-supplied hello bytes and drop notify_finished ✨ * fix(supervisor): load precompiled modules only for registry runners 🔒️ * fix(fees): stop a zero execution budget aborting internally 🐛 * chore(exe): drop the unreachable dlsym linker helper 🔥 * test(integration): add routed, unservable-major and mismatch cases ✅
kp2pml30
force-pushed
the
pr/v0.3/feat/rework-manager-api
branch
from
July 29, 2026 13:13
30fcc9e to
96e7581
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
executor/src/wasi/genlayer_sdk/tests.rs (1)
166-178: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider covering the other
nested_run_okrejection paths.
small_hash.len() != 32,ResultCode::InternalError, and the non-stringVmErrorpayload are all invariants worth pinning, since they guard cross-executor hash agreement.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@executor/src/wasi/genlayer_sdk/tests.rs` around lines 166 - 178, Expand the tests around nested_run_ok to cover each remaining rejection path: a small_hash whose length is not 32, a nested result with ResultCode::InternalError, and a VmError result carrying a non-string payload. Assert that nested_run_ok returns an error for each case, reusing the existing NestedRunReply setup and keeping the effect_free rejection test unchanged.executor/src/rt/vm/storage.rs (1)
485-494: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
check_majordoesn't useSelf/HS— consider a free function.Being an inherent associated fn of
Storage<HS>forces callers into an unrelated turbofish (Storage::<StorageHostHolder>::check_major(..)inexecutor/src/wasi/genlayer_sdk/run.rslines 148 and 501). A module-levelpub fn check_major(contract_major: u8)inrt::vm::storagewould read better and drop the bogus type parameter.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@executor/src/rt/vm/storage.rs` around lines 485 - 494, Move check_major out of the Storage<HS> inherent implementation into a module-level public function in rt::vm::storage, since it does not use Self or HS. Update all callers, including the usages in run.rs, to invoke the module function without the Storage::<...> turbofish while preserving the existing validation behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@executor/src/wasi/genlayer_sdk/run.rs`:
- Around line 188-199: Update the nested protocol’s ResultCode::VmError handling
in the run flow to normalize and validate the materialized error code before
constructing public_abi::VmError. Reuse the existing strip_vm_error_detail and
validate_leader_vm_error helpers in this file, preserving the existing rejection
of non-string data and ensuring only canonical VM error codes reach the caller
and determinant fingerprint.
In `@support/scripts/check-source-text.py`:
- Around line 37-40: Extend the source-policy regexes in the check-source-text
logic to detect unqualified libc environment mutations, including direct calls,
imported symbols such as use libc::setenv, and aliased imports/calls for setenv,
unsetenv, and putenv. Add regression tests covering qualified, imported, and
aliased forms while preserving the existing env and libc-qualified matches.
---
Nitpick comments:
In `@executor/src/rt/vm/storage.rs`:
- Around line 485-494: Move check_major out of the Storage<HS> inherent
implementation into a module-level public function in rt::vm::storage, since it
does not use Self or HS. Update all callers, including the usages in run.rs, to
invoke the module function without the Storage::<...> turbofish while preserving
the existing validation behavior.
In `@executor/src/wasi/genlayer_sdk/tests.rs`:
- Around line 166-178: Expand the tests around nested_run_ok to cover each
remaining rejection path: a small_hash whose length is not 32, a nested result
with ResultCode::InternalError, and a VmError result carrying a non-string
payload. Assert that nested_run_ok returns an error for each case, reusing the
existing NestedRunReply setup and keeping the effect_free rejection test
unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: e0b170c9-9185-4b4e-bc8c-0489152c49b3
⛔ Files ignored due to path filters (56)
executor/Cargo.lockis excluded by!**/*.lock,!**/*.lockexecutor/crates/common/Cargo.lockis excluded by!**/*.lock,!**/*.lockexecutor/crates/modules-interfaces/Cargo.lockis excluded by!**/*.lock,!**/*.locktests/integration/claude/AGENT_PROMPT.mdis excluded by!tests/**tests/integration/claude/README.mdis excluded by!tests/**tests/integration/claude/agent/float_math/README.mdis excluded by!tests/**tests/integration/claude/agent/float_math/contract.pyis excluded by!tests/**tests/integration/claude/agent/float_math/float_math.jsonnetis excluded by!tests/**tests/integration/claude/example/README.mdis excluded by!tests/**tests/integration/claude/example/example.jsonnetis excluded by!tests/**tests/integration/claude/intelligence/EXPLORED_PATHS.mdis excluded by!tests/**tests/integration/claude/intelligence/TODO.mdis excluded by!tests/**tests/integration/claude/run-manager.shis excluded by!tests/**tests/integration/claude/run_test.pyis excluded by!tests/**tests/integration/exploit/disagree_in_sandbox/disagree_in_sandbox.jsonnetis excluded by!tests/**tests/integration/exploit/unservable_major/unservable_major.0.stdoutis excluded by!**/*.stdout,!tests/**tests/integration/exploit/unservable_major/unservable_major.0_0.stdoutis excluded by!**/*.stdout,!tests/**tests/integration/exploit/unservable_major/unservable_major.0_0_0.stdoutis excluded by!**/*.stdout,!tests/**tests/integration/exploit/unservable_major/unservable_major.1.stdoutis excluded by!**/*.stdout,!tests/**tests/integration/exploit/unservable_major/unservable_major.1_0.stdoutis excluded by!**/*.stdout,!tests/**tests/integration/exploit/unservable_major/unservable_major.1_0_0.stdoutis excluded by!**/*.stdout,!tests/**tests/integration/exploit/unservable_major/unservable_major.jsonnetis excluded by!tests/**tests/integration/exploit/unservable_major/unservable_major_from.pyis excluded by!tests/**tests/integration/exploit/unservable_major/unservable_major_to.pyis excluded by!tests/**tests/integration/hello-world/hello_world/hello_world.0.stdoutis excluded by!**/*.stdout,!tests/**tests/integration/hello-world/hello_world/hello_world.jsonnetis excluded by!tests/**tests/integration/hello-world/hello_world/hello_world_trivial.pyis excluded by!tests/**tests/integration/hello-world/hello_world_class_nondet/hello_world_class_nondet.0.stdoutis excluded by!**/*.stdout,!tests/**tests/integration/hello-world/hello_world_class_nondet/hello_world_class_nondet.jsonnetis excluded by!tests/**tests/integration/hello-world/hello_world_class_nondet/hello_world_class_nondet.pyis excluded by!tests/**tests/integration/message/use_balance_zero_budget/use_balance_zero_budget.0.stdoutis excluded by!**/*.stdout,!tests/**tests/integration/message/use_balance_zero_budget/use_balance_zero_budget.0_0.stdoutis excluded by!**/*.stdout,!tests/**tests/integration/message/use_balance_zero_budget/use_balance_zero_budget.jsonnetis excluded by!tests/**tests/integration/message/use_balance_zero_budget/use_balance_zero_budget.pyis excluded by!tests/**tests/integration/misc/routed_call/routed_call.0.stdoutis excluded by!**/*.stdout,!tests/**tests/integration/misc/routed_call/routed_call.0_0.stdoutis excluded by!**/*.stdout,!tests/**tests/integration/misc/routed_call/routed_call.0_0_0.stdoutis excluded by!**/*.stdout,!tests/**tests/integration/misc/routed_call/routed_call.jsonnetis excluded by!tests/**tests/integration/misc/routed_call/routed_call_from.pyis excluded by!tests/**tests/integration/misc/routed_call/routed_call_to.pyis excluded by!tests/**tests/integration/nasty-determinism/floats/contract.pyis excluded by!tests/**tests/integration/nasty-determinism/floats/floats.0.stdoutis excluded by!**/*.stdout,!tests/**tests/integration/nasty-determinism/floats/floats.jsonnetis excluded by!tests/**tests/integration/runner/absent_runner_id/absent_runner_id.0.stdoutis excluded by!**/*.stdout,!tests/**tests/integration/runner/absent_runner_id/absent_runner_id.0_0.stdoutis excluded by!**/*.stdout,!tests/**tests/integration/runner/absent_runner_id/absent_runner_id.0_1.stdoutis excluded by!**/*.stdout,!tests/**tests/integration/runner/absent_runner_id/absent_runner_id.0_2.stdoutis excluded by!**/*.stdout,!tests/**tests/integration/runner/absent_runner_id/absent_runner_id.jsonnetis excluded by!tests/**tests/integration/runner/absent_runner_id/absent_runner_id.pyis excluded by!tests/**tests/integration/runner/major_mismatch/major_mismatch.3.stdoutis excluded by!**/*.stdout,!tests/**tests/integration/runner/major_mismatch/major_mismatch.3_0.stdoutis excluded by!**/*.stdout,!tests/**tests/integration/runner/major_mismatch/major_mismatch.jsonnetis excluded by!tests/**tests/integration/storage/hello_world_class/hello_world_class.0.stdoutis excluded by!**/*.stdout,!tests/**tests/integration/storage/hello_world_class/hello_world_class.0_0.stdoutis excluded by!**/*.stdout,!tests/**tests/integration/storage/hello_world_class/hello_world_class.jsonnetis excluded by!tests/**tests/integration/storage/hello_world_class/hello_world_class.pyis excluded by!tests/**
📒 Files selected for processing (31)
.genvm-tool.py.gitignoreexecutor/Cargo.tomlexecutor/codegen/data/host-fns.jsonexecutor/codegen/data/public-abi.jsonexecutor/crates/common/src/expr/evaluator.rsexecutor/crates/common/src/expr/lexer.rsexecutor/crates/common/src/expr/tokenizer.rsexecutor/crates/common/src/expr/value.rsexecutor/crates/common/src/host_fns.rsexecutor/crates/common/src/lib.rsexecutor/crates/common/tests/expr.rsexecutor/crates/modules-interfaces/Cargo.tomlexecutor/src/exe/precompile.rsexecutor/src/exe/run.rsexecutor/src/host/mod.rsexecutor/src/lib.rsexecutor/src/rt/memlimiter.rsexecutor/src/rt/mod.rsexecutor/src/rt/supervisor/actions.rsexecutor/src/rt/supervisor/mod.rsexecutor/src/rt/vm/mod.rsexecutor/src/rt/vm/storage.rsexecutor/src/wasi/genlayer_sdk/message.rsexecutor/src/wasi/genlayer_sdk/mod.rsexecutor/src/wasi/genlayer_sdk/run.rsexecutor/src/wasi/genlayer_sdk/tests.rsexecutor/src/wasi/mod.rsexecutor/tests/precompile_lookup.rsflake.nixsupport/scripts/check-source-text.py
💤 Files with no reviewable changes (3)
- executor/codegen/data/host-fns.json
- executor/crates/common/src/host_fns.rs
- executor/src/wasi/mod.rs
Comment on lines
+188
to
+199
| let run_ok = match reply.result.kind { | ||
| genvm_modules_interfaces::ResultCode::Return => rt::vm::RunOk::Return(reply.result.data), | ||
| genvm_modules_interfaces::ResultCode::UserError => { | ||
| rt::vm::RunOk::UserError(reply.result.data) | ||
| } | ||
| genvm_modules_interfaces::ResultCode::VmError => { | ||
| let data = reply.result.data.materialize()?; | ||
| let calldata::Value::Str(code) = data else { | ||
| anyhow::bail!("nested CallContract VM error is not a string"); | ||
| }; | ||
| rt::vm::RunOk::VMError(public_abi::VmError(std::borrow::Cow::Owned(code)), None) | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Nested VmError codes are trusted verbatim.
The in-process/leader paths normalize and validate VM error codes (strip_vm_error_detail / validate_leader_vm_error in this file), but a code arriving over the nested protocol is wrapped into public_abi::VmError unchecked. An unknown or detail-carrying string then reaches the caller and the det fingerprint, and can trip the debug_assert! in strip_vm_error_detail if it later flows through that path. Consider validating/stripping here too.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@executor/src/wasi/genlayer_sdk/run.rs` around lines 188 - 199, Update the
nested protocol’s ResultCode::VmError handling in the run flow to normalize and
validate the materialized error code before constructing public_abi::VmError.
Reuse the existing strip_vm_error_detail and validate_leader_vm_error helpers in
this file, preserving the existing rejection of non-string data and ensuring
only canonical VM error codes reach the caller and determinant fingerprint.
Comment on lines
+37
to
+40
| re.compile(r'\benv::(?:set_var|remove_var)\b'), | ||
| re.compile(r'\blibc::(?:setenv|unsetenv|putenv)\b'), | ||
| # ... including when imported, which hides the `env::` prefix at the call. | ||
| re.compile(r'\buse\s+(?:std::)?env::\{?[^;]*\b(?:set_var|remove_var)\b'), |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Cover imported and unqualified libc environment mutations.
The libc pattern only matches libc::setenv, libc::unsetenv, and libc::putenv. It misses common forms such as use libc::setenv; setenv(...) and direct setenv(...) calls, so prohibited environment mutation can bypass this source-policy check. Add matching patterns and regression tests for qualified, imported, and aliased calls.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@support/scripts/check-source-text.py` around lines 37 - 40, Extend the
source-policy regexes in the check-source-text logic to detect unqualified libc
environment mutations, including direct calls, imported symbols such as use
libc::setenv, and aliased imports/calls for setenv, unsetenv, and putenv. Add
regression tests covering qualified, imported, and aliased forms while
preserving the existing env and libc-qualified matches.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Auto-opened executor mirror of genlayerlabs/genvm-manager#9.
Carries the executor-side work for that manager PR. Auto-closed as merged when the manager PR lands (its
pr/v0.3/feat/rework-manager-apibranch is moved ontov0.3-dev).Summary by CodeRabbit
trueandfalsevalues in expressions, including comparisons, conditions, collections, and bindings.